Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fix](vault) Fix bugs about altering storage vault name #45685

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[fix](vault) Fix bugs about altering storage vault name #45685

wants to merge 1 commit into from

Conversation

SWJTU-ZhangLei
Copy link
Contributor

@SWJTU-ZhangLei SWJTU-ZhangLei commented Dec 20, 2024 •
edited
Loading

  • fix altering storage name but not writing disk in meta-service
  • check vault if existed when altering stoarge vault name

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen
Copy link
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@SWJTU-ZhangLei
Copy link
Contributor Author

run buildall

github-actions[bot]
github-actions bot reviewed Dec 20, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

cloud/src/meta-service/meta_service_resource.cpp
return false;
}

static int alter_hdfs_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction>& txn,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'alter_hdfs_storage_vault' exceeds recommended size/complexity thresholds [readability-function-size]

static int alter_hdfs_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction>& txn,
           ^
Additional context

cloud/src/meta-service/meta_service_resource.cpp:534: 116 lines including whitespace and comments (threshold 80)

static int alter_hdfs_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction>& txn,
           ^

cloud/src/meta-service/meta_service_resource.cpp
return 0;
}

static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction> txn,
static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction>& txn,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'alter_s3_storage_vault' exceeds recommended size/complexity thresholds [readability-function-size]

static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction>& txn,
           ^
Additional context

cloud/src/meta-service/meta_service_resource.cpp:654: 130 lines including whitespace and comments (threshold 80)

static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptr<Transaction>& txn,
           ^

@doris-robot
Copy link

TPC-H: Total hot run time: 39897 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 8c448aa3683dd8035f657bd3563dd68b49bf2013, data reload: false

------ Round 1 ----------------------------------
q1	17682	7550	7265	7265
q2	2051	188	174	174
q3	10622	1094	1218	1094
q4	10590	745	806	745
q5	7623	2764	2650	2650
q6	241	150	146	146
q7	984	615	591	591
q8	9234	1900	1961	1900
q9	6639	6467	6430	6430
q10	6994	2321	2321	2321
q11	463	287	263	263
q12	436	226	228	226
q13	17793	2968	3022	2968
q14	246	210	212	210
q15	589	520	508	508
q16	697	598	586	586
q17	975	547	542	542
q18	7407	6803	6610	6610
q19	1362	1032	1003	1003
q20	476	184	186	184
q21	4055	3253	3172	3172
q22	383	317	309	309
Total cold run time: 107542 ms
Total hot run time: 39897 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7259	7224	7235	7224
q2	330	220	228	220
q3	2896	2825	2994	2825
q4	2060	1840	1846	1840
q5	5680	5680	5680	5680
q6	225	149	150	149
q7	2221	1843	1869	1843
q8	3384	3545	3586	3545
q9	8908	9098	9046	9046
q10	3585	3550	3565	3550
q11	586	514	506	506
q12	830	614	623	614
q13	14224	3182	3085	3085
q14	326	274	283	274
q15	577	492	500	492
q16	703	641	656	641
q17	1856	1638	1631	1631
q18	8238	7736	7668	7668
q19	1714	1591	1466	1466
q20	2108	1890	1848	1848
q21	5783	5484	5387	5387
q22	657	582	587	582
Total cold run time: 74150 ms
Total hot run time: 60116 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 196967 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 8c448aa3683dd8035f657bd3563dd68b49bf2013, data reload: false

query1	1290	967	902	902
query2	6238	2366	2361	2361
query3	11091	4910	4877	4877
query4	33473	23422	23604	23422
query5	5096	469	452	452
query6	284	198	195	195
query7	3996	315	319	315
query8	318	285	254	254
query9	9536	2694	2707	2694
query10	520	261	250	250
query11	18158	15225	15300	15225
query12	162	109	105	105
query13	1670	423	420	420
query14	11548	7280	7493	7280
query15	286	188	191	188
query16	8144	496	458	458
query17	1736	627	618	618
query18	2204	352	299	299
query19	379	162	166	162
query20	127	120	110	110
query21	217	107	112	107
query22	4798	4507	4418	4418
query23	39299	34386	33604	33604
query24	10599	2573	2483	2483
query25	515	395	389	389
query26	740	161	151	151
query27	2651	325	347	325
query28	7636	2461	2480	2461
query29	682	420	411	411
query30	234	168	150	150
query31	1046	835	812	812
query32	95	57	60	57
query33	787	297	314	297
query34	1045	529	517	517
query35	866	796	770	770
query36	1124	949	948	948
query37	124	72	85	72
query38	4216	4212	4225	4212
query39	1534	1456	1434	1434
query40	207	125	102	102
query41	45	50	44	44
query42	122	104	106	104
query43	534	515	500	500
query44	1316	845	841	841
query45	199	167	172	167
query46	1205	709	738	709
query47	2052	1922	1893	1893
query48	423	343	326	326
query49	943	394	418	394
query50	826	394	399	394
query51	7274	7076	7183	7076
query52	110	96	90	90
query53	270	192	197	192
query54	1077	408	438	408
query55	80	80	81	80
query56	284	276	269	269
query57	1279	1173	1181	1173
query58	247	216	239	216
query59	3334	3268	3200	3200
query60	281	253	252	252
query61	106	107	113	107
query62	839	692	701	692
query63	222	194	201	194
query64	3883	745	681	681
query65	3341	3264	3261	3261
query66	681	302	307	302
query67	16502	15606	15472	15472
query68	6750	555	535	535
query69	486	255	255	255
query70	1256	1131	1147	1131
query71	524	274	257	257
query72	6758	4071	4127	4071
query73	800	357	365	357
query74	9963	8888	8855	8855
query75	4156	2686	2637	2637
query76	4616	1205	1105	1105
query77	812	287	287	287
query78	10320	9406	9297	9297
query79	1887	608	600	600
query80	872	431	419	419
query81	527	236	234	234
query82	702	119	118	118
query83	239	146	144	144
query84	265	71	72	71
query85	1234	306	294	294
query86	406	307	301	301
query87	4539	4403	4326	4326
query88	3858	2203	2184	2184
query89	440	293	290	290
query90	1977	188	190	188
query91	143	100	105	100
query92	60	53	58	53
query93	2521	556	538	538
query94	663	300	364	300
query95	346	252	246	246
query96	630	284	275	275
query97	2846	2673	2633	2633
query98	217	192	196	192
query99	1654	1330	1331	1330
Total cold run time: 314613 ms
Total hot run time: 196967 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 33.53 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 8c448aa3683dd8035f657bd3563dd68b49bf2013, data reload: false

query1	0.03	0.02	0.03
query2	0.10	0.04	0.05
query3	0.23	0.06	0.05
query4	1.64	0.08	0.08
query5	0.41	0.41	0.41
query6	1.14	0.66	0.65
query7	0.02	0.01	0.03
query8	0.05	0.05	0.04
query9	0.56	0.49	0.50
query10	0.57	0.59	0.55
query11	0.16	0.12	0.13
query12	0.17	0.13	0.12
query13	0.61	0.60	0.60
query14	2.74	2.73	2.77
query15	0.91	0.84	0.83
query16	0.37	0.38	0.37
query17	1.08	1.07	1.06
query18	0.19	0.19	0.19
query19	2.00	1.88	2.07
query20	0.01	0.01	0.01
query21	15.36	0.65	0.64
query22	4.27	7.61	1.98
query23	18.29	1.46	1.39
query24	2.21	0.21	0.21
query25	0.15	0.09	0.08
query26	0.27	0.19	0.17
query27	0.08	0.08	0.08
query28	13.35	1.16	1.17
query29	12.64	3.38	3.38
query30	0.25	0.07	0.07
query31	2.82	0.41	0.41
query32	3.24	0.48	0.48
query33	3.15	3.11	3.07
query34	16.86	4.54	4.51
query35	4.60	4.56	4.55
query36	0.67	0.50	0.48
query37	0.20	0.16	0.16
query38	0.16	0.15	0.15
query39	0.06	0.04	0.04
query40	0.17	0.13	0.13
query41	0.10	0.05	0.05
query42	0.06	0.05	0.05
query43	0.06	0.05	0.04
Total cold run time: 112.01 s
Total hot run time: 33.53 s

@SWJTU-ZhangLei
[fix](vault) Fix bugs about altering storage vault name
d5cd146 
* fix altering storage name but not writing disk in meta-service
* check vault if existed when altering stoarge vault name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@dataroaring dataroaring Awaiting requested review from dataroaring dataroaring will be requested when the pull request is marked ready for review dataroaring is a code owner

@CalvinKirs CalvinKirs Awaiting requested review from CalvinKirs CalvinKirs will be requested when the pull request is marked ready for review CalvinKirs is a code owner

@morningman morningman Awaiting requested review from morningman morningman will be requested when the pull request is marked ready for review morningman is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SWJTU-ZhangLei @hello-stephen @doris-robot